[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

DarshitChanpura · 2025-03-17T19:57:20Z

#5016 is being broken down into smaller pieces. This is part 1.

Description

Introduces an SPI to implement Resource Access Control in OpenSearch. This will allow plugins to declare itself as Resource Plugins to leverage the access control methods to be provided by Security plugin.

Category - New feature
Why these changes are required?
- At present, plugins have implemented in-house authorization mechanisms for a lack of centralized framework. This PR introduces a centralized way to offload resource permissions check to security plugin.

Issues Resolved

Related to #4500

Testing

automated tests

Check List

~~- [ ] New functionality includes testing~~
~~- [ ] New functionality has been documented~~
~- [ ] New Roles/Permissions have a corresponding security dashboards plugin PR
~~- [ ] API changes companion pull request created~~

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2025-03-17T21:06:47Z

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.69%. Comparing base (bfc0f4a) to head (c9e2485).

Files with missing lines	Patch %	Lines
.../opensearch/security/OpenSearchSecurityPlugin.java	66.66%	1 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                        @@
##           feature/resource-permissions    #5185      +/-   ##
================================================================
- Coverage                         71.71%   71.69%   -0.03%     
================================================================
  Files                               337      337              
  Lines                             22789    22790       +1     
  Branches                           3606     3606              
================================================================
- Hits                              16344    16340       -4     
- Misses                             4649     4653       +4     
- Partials                           1796     1797       +1

Files with missing lines	Coverage Δ
...g/opensearch/security/support/ConfigConstants.java	`95.23% <ø> (ø)`
.../opensearch/security/OpenSearchSecurityPlugin.java	`83.72% <66.66%> (-0.12%)`	⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java

spi/README.md

spi/src/main/java/org/opensearch/security/spi/resources/ResourceSharingExtension.java

Signed-off-by: Darshit Chanpura <[email protected]>

.gitignore

spi/README.md

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java

Signed-off-by: Darshit Chanpura <[email protected]>

nibix

Thanks for this interesting PR @DarshitChanpura !

Like in #5194, I have added a couple of comments and questions. As I went through the code linearly, I also commented both on conceptual things and code level things (which I mostly marked with Nit:). I would recommend that you first have a look on conceptual things ... we can talk about the Nit stuff later.

spi/README.md

...src/main/java/org/opensearch/security/spi/resources/exceptions/ResourceSharingException.java

spi/src/main/java/org/opensearch/security/spi/resources/sharing/Creator.java

nibix · 2025-03-21T05:49:12Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java

+public enum Recipient {
+    USERS("users"),
+    ROLES("roles"),
+    BACKEND_ROLES("backend_roles");


I know that some plugins use backend roles for sharing. However, this might lead to sub-optimal situations, as users might have different backend roles depending on the auth system they are using. If we need to support BACKEND_ROLES, it should be marked IMHO as deprecated with the suggestion to use ROLES instead.

BACKEND_ROLES are added to support transition from filterByBackendRoles authz mechanism to this. This will allow migration to be easier. Maybe we deprecate this in next iteration of this feature?

Just deprecating it won't really affect migration negatively. It would just send the message "in the future, please use other means". Or do I miss something?

This can be decided in v2 of this feature. For now, the idea is to centralize the resource authz mechanism.

spi/src/main/java/org/opensearch/security/spi/resources/sharing/RecipientType.java

spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java

spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java

spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithScope.java

spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java

DarshitChanpura · 2025-03-21T20:05:32Z

Proposal on path forward for scopes:

To add to the comment here,

Scopes can be thought of as placeholders for action groups. The question here is whether we should include Scopes in this iteration of the feature, at all. From what I see there are 3 paths forward:

Path 1: No Scopes

{
	"share_with": {
		"users": [...],
		"roles": [...],
		"backend_roles": [...]
	}
}

With this structure there is no concept of "scope". The document will be visible on 3 levels: Private, Restricted and Public. Here is what a sample resource-sharing document will look like in each case:

Private

share_with record can be empty or not be present at all.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { }
}

Restricted

share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": {
		"users": [...],
		"roles": [...],
		"backend_roles": [...]
	}
}

Public:

share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": {
		"users": ["*"],
		"roles": ["*"],
		"backend_roles": ["*"]
	}
}

Pros:

No confusion around scopes

Cons:

No access control of any specific type. It relies on APIs to be defined clearly and users to be mapped correctly.
Requires another plugin migration when we introduce Resource Authorization Framework as a standalone authorization mechanism

Path 2: Placeholder for Scopes

{
	"share_with": {
		"*" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
	}
}

With this structure we introduce a default scope "" which acts as a place holder for action-groups to be declared when a standalone Resource Authorization framework is in place. "" allows all actions to be matched.

Private

* record inside share_with can be empty or not be present at all

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*" : { }
	} 
}

Restricted

* record inside share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*": {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
	}
}

Public:

* record inside share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*": {
			"users": ["*"],
			"roles": ["*"],
			"backend_roles": ["*"]
		}
	}
}

Pros:

No confusion around scopes.
Will not require migration since future sharing info can be added on top this ("*" still retains its meaning of all actions being allowed, but an additional read_only action-group can be added to allow read-only access)

Cons:

Any action can be allowed since it is "*".

Path 3: Continue with scopes

This approach allows defining individual access type over each resource. It doesn't rely completely on the APIs. It can be in future expanded to be mapped to action groups when resource authorization framework is implemented.

Example, AD plugin can, at present, define a scope named ad_read_only and in future an action-group can be added by the name of ad_read_only which contains a set of actions to be utilized by Resource Authorization framework.

Here is what an example detector sharing record may look like:

{
	"resource_id": "detector1",
	"source_idx": ".opendistro-anomaly-detectors",
	"created_by": {
		"user": "darshit"
	},
	"share_with": { 
		"ad_read_only" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		},
		"ad_full_access" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
		
	}
}

or if the detector is intended to be publicly accessible:

{
	"resource_id": "detector1",
	"source_idx": ".opendistro-anomaly-detectors",
	"created_by": {
		"user": "darshit"
	},
	"share_with": { 
		"*" : {
			"users": ["*"],
			"roles": ["*"],
			"backend_roles": ["*"]
		}
	}
}

Pros:

No future migration

Cons:

Complex to understand at first.
Requires plugin to pass scopes, when asking for access verification. What scopes should current user's access be matched against.

@nibix @cwperks @derek-ho Please review this proposal so we can finalize the path-forward for scopes.

I lean towards Path 2 as it is good place between no scopes and complete scopes, and it unblocks this PR. It would require some re-work in following Part 2 and 3 tho.

DarshitChanpura · 2025-03-21T20:18:55Z

Proposal on path forward for scopes:
@nibix @cwperks @derek-ho Please review this proposal so we can finalize the path-forward for scopes.

I personally am leaning towards Path 2 as it is good place between no scopes and complete scopes.

Path 3 puts us in a good state for future, but I don't want that to be a blocker for this PR. All changes are labelled as @opensearch.experimental so that gives us room to modify things later.

Signed-off-by: Darshit Chanpura <[email protected]>

nibix · 2025-03-22T08:51:20Z

@DarshitChanpura

Proposal on path forward for scopes:

Thank you for the proposals!

Before I am going into the proposals, I'd like to go one step back and try to get a better picture of the situation. Sorry if the following question may sound dumb, but I am possibly missing some details at the moment.

So, my question would be: What's the actual reason we are now discussing these options about the path forward with scopes?

Is it because there is disagreement about the scopes?
Is it because the scopes are not sufficiently/thoroughly defined yet?
Or something else?

I think an answer to this question could help me to contribute better to this discussion.

cwperks · 2025-03-24T14:01:28Z

Is it because there is disagreement about the scopes?

Not conceptually, more on implementation. @DarshitChanpura has introduced a notion of "scopes" and can elaborate on them and their implementation. At the onset of this project, I had imagined re-using action groups since its an existing mechanism to group an enumerable list of actions together.

Conceptually, a "scope" is a group of actions that can be performed on a resource and the owner of the resource gets to specify the level of access when sharing. For an example, we use a tool for doc reviews that has 4 different access levels when sharing a document. The access levels are "Read Only", "Read + Comment", "Edit" (allows read, comment, edit, but not share) and "Full Access" which includes the ability to share a document. These are examples of what the scopes would be for a plugin.

FYI scopes (or action groups) for resource authorization breaks with the current security model where the cluster administrator is the one assigning permissions.

I think alerting is a good example of a plugin that has multiple levels of access that could benefit from more fine-grained access on sharable resources. They already define 3 default roles to include in the default distribution.

DarshitChanpura · 2025-03-24T15:03:51Z

Yes, there is some confusion around implementation of scopes.

Scopes do not equate to any action-groups or resolve to any set of actions, at the moment in this PR. They are added to allow resources to be shared with an additional layer of security, meaning even if a user possesses access to, say a GET detector API, they would additionally need READ scope access. This sets us in a good state when Resource Authz framework is implemented as a standalone framework (with Resource Request and ResourcePrivEvaluator). At the moment, Resource Authz works on top of existing authz framework.

To avoid, any sort of confusion, I've put in a proposal for paths forward for this feature. The idea is to at least address ML-commons use case where they declare three types of access to resources: Private, Restricted and Public (as described in Paths 1 and 2 above). This allows us to achieve the final implementation of scopes (or action-groups) when we implement the standalone framework.

I believe Path 2 is a good setup which is forward-looking and also resolves Ml-commons use case.

nibix · 2025-03-24T16:47:19Z

@cwperks @DarshitChanpura

Thank you for your replies!

I think a part of my personal confusion is caused by the fact that the word "scope" can mean so many things. It could also mean the audience a resource is shared with - that was actually my first intuition. It took me a while to find that this is wrong.

So, IMHO, I'd hope that a lot of the confusion could be resolved by finding another name for scopes, which is more precise and more intuitive. What do you think?

DarshitChanpura · 2025-03-24T19:14:27Z

I think a part of my personal confusion is caused by the fact that the word "scope" can mean so many things. It could also mean the audience a resource is shared with - that was actually my first intuition. It took me a while to find that this is wrong.

Scope is the level at which the resource is shared.

So, IMHO, I'd hope that a lot of the confusion could be resolved by finding another name for scopes, which is more precise and more intuitive. What do you think?

Yes that can be done. However, the important question here is which path to proceed with, out of the three.

Signed-off-by: Darshit Chanpura <[email protected]>

…into resource-sharing-spi-only

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura · 2025-03-26T21:30:39Z

I've update this PR and subsequent parts to reflect action-groups change following Path 2 from design proposals put forward here: #5185 (comment)

Signed-off-by: Darshit Chanpura <[email protected]>

...rc/main/java/org/opensearch/security/spi/resources/exceptions/ResourceNotFoundException.java

Signed-off-by: Darshit Chanpura <[email protected]>

derek-ho

Generally LGTM from a high level, I will take a look at part 2 and 3 to see if there is anything else concerning on this PR

DarshitChanpura changed the title ~~[Resource Access Control] Introduces SPI for resource access control~~ [Resource Access Control] [Part1] Introduces SPI for resource access control Mar 17, 2025

DarshitChanpura mentioned this pull request Mar 17, 2025

[Resource Access Control] [Part2] Introduces a client for Resource Access Control and adds concrete implementation via common package #5186

Closed

4 tasks

DarshitChanpura force-pushed the resource-sharing-spi-only branch 2 times, most recently from d076506 to ccc5022 Compare March 17, 2025 21:21

DarshitChanpura changed the base branch from feature/resource-permissions to main March 17, 2025 21:23

DarshitChanpura changed the base branch from main to feature/resource-permissions March 17, 2025 21:23

DarshitChanpura mentioned this pull request Mar 17, 2025

[Resource Access Control] [Part3] Introduces a sample plugin to demonstrate usage of Resource Access Control feature #5187

Draft

3 tasks

DarshitChanpura marked this pull request as ready for review March 17, 2025 22:42

DarshitChanpura requested review from cwperks, derek-ho, nibix, peternied, RyanL1997, reta and willyborankin as code owners March 17, 2025 22:42

This was referenced Mar 17, 2025

[Feature] Introduces Centralized Resource Access Control and Sharing #5016

Closed

[Resource Access Control] [Cleanup] Removes redundant files between common and root package #5188

Closed

DarshitChanpura added the resource-permissions Label to track all items related to resource permissions label Mar 18, 2025

cwperks reviewed Mar 19, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java Outdated Show resolved Hide resolved

cwperks reviewed Mar 19, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java Outdated Show resolved Hide resolved

cwperks reviewed Mar 20, 2025

View reviewed changes

spi/README.md Outdated Show resolved Hide resolved

cwperks reviewed Mar 20, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/ResourceSharingExtension.java Outdated Show resolved Hide resolved

DarshitChanpura mentioned this pull request Mar 20, 2025

[Resource Access Control] [Part2] Introduces a client for Resource Access Control and adds concrete implementation for resource access control #5194

Open

4 tasks

DarshitChanpura added 2 commits March 20, 2025 14:19

Introduces resource sharing and access control SPI

77d92f7

Signed-off-by: Darshit Chanpura <[email protected]>

Renames couple of SPI classes

3379974

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from 9833c7e to 3379974 Compare March 20, 2025 18:19

derek-ho reviewed Mar 20, 2025

View reviewed changes

.gitignore Show resolved Hide resolved

derek-ho reviewed Mar 20, 2025

View reviewed changes

spi/README.md Show resolved Hide resolved

derek-ho reviewed Mar 20, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java Outdated Show resolved Hide resolved

Fixes CI

63c4a27

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from cb45e7e to 63c4a27 Compare March 20, 2025 20:37

Fixes test

2fd2a8b

Signed-off-by: Darshit Chanpura <[email protected]>

nibix reviewed Mar 21, 2025

View reviewed changes

derek-ho reviewed Mar 21, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java Outdated Show resolved Hide resolved

Adds new exception types and updated README.md

eaa92b0

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from fa10c26 to eaa92b0 Compare March 21, 2025 22:17

DarshitChanpura added 4 commits March 26, 2025 17:08

Refactors scopes to action-groups and updates SPI

f3d13ee

Signed-off-by: Darshit Chanpura <[email protected]>

Merge remote-tracking branch 'upstream/feature/resource-permissions' …

c35717b

…into resource-sharing-spi-only

Fixes build.gradle

806d7a5

Signed-off-by: Darshit Chanpura <[email protected]>

Updates failureaccess dep

a0d923d

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura mentioned this pull request Mar 26, 2025

[Proposal] Resource Permissions and Sharing #4500

Open

5 tasks

Fixes build-artifacts workflow

dafd5c3

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from 6b218d9 to dafd5c3 Compare March 26, 2025 21:44

DarshitChanpura added 2 commits March 26, 2025 18:52

Removes recipient type registry logic

5738e11

Signed-off-by: Darshit Chanpura <[email protected]>

Updates an exception class

ef7bb47

Signed-off-by: Darshit Chanpura <[email protected]>

nibix reviewed Mar 27, 2025

View reviewed changes

...rc/main/java/org/opensearch/security/spi/resources/exceptions/ResourceNotFoundException.java Outdated Show resolved Hide resolved

DarshitChanpura added 3 commits March 27, 2025 15:55

Removes custom resource-sharing exception classes

a32b32c

Signed-off-by: Darshit Chanpura <[email protected]>

Updates spi README

0798fea

Signed-off-by: Darshit Chanpura <[email protected]>

Updates forced dependencies

c9e2485

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from 44739a6 to c9e2485 Compare March 28, 2025 17:51

derek-ho approved these changes Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

DarshitChanpura commented Mar 17, 2025 •

edited

Loading

codecov bot commented Mar 17, 2025 •

edited

Loading

nibix left a comment

nibix Mar 21, 2025

DarshitChanpura Mar 21, 2025

nibix Mar 25, 2025

DarshitChanpura Mar 26, 2025

DarshitChanpura commented Mar 21, 2025 •

edited

Loading

Path 1: No Scopes

Path 2: Placeholder for Scopes

Path 3: Continue with scopes

DarshitChanpura commented Mar 21, 2025

nibix commented Mar 22, 2025 •

edited

Loading

cwperks commented Mar 24, 2025 •

edited

Loading

DarshitChanpura commented Mar 24, 2025

nibix commented Mar 24, 2025 •

edited

Loading

DarshitChanpura commented Mar 24, 2025

DarshitChanpura commented Mar 26, 2025

derek-ho left a comment

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

Are you sure you want to change the base?

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

Conversation

DarshitChanpura commented Mar 17, 2025 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov bot commented Mar 17, 2025 • edited Loading

Codecov Report

nibix left a comment

Choose a reason for hiding this comment

nibix Mar 21, 2025

Choose a reason for hiding this comment

DarshitChanpura Mar 21, 2025

Choose a reason for hiding this comment

nibix Mar 25, 2025

Choose a reason for hiding this comment

DarshitChanpura Mar 26, 2025

Choose a reason for hiding this comment

DarshitChanpura commented Mar 21, 2025 • edited Loading

Path 1: No Scopes

Path 2: Placeholder for Scopes

Path 3: Continue with scopes

DarshitChanpura commented Mar 21, 2025

nibix commented Mar 22, 2025 • edited Loading

cwperks commented Mar 24, 2025 • edited Loading

DarshitChanpura commented Mar 24, 2025

nibix commented Mar 24, 2025 • edited Loading

DarshitChanpura commented Mar 24, 2025

DarshitChanpura commented Mar 26, 2025

derek-ho left a comment

Choose a reason for hiding this comment

DarshitChanpura commented Mar 17, 2025 •

edited

Loading

codecov bot commented Mar 17, 2025 •

edited

Loading

DarshitChanpura commented Mar 21, 2025 •

edited

Loading

nibix commented Mar 22, 2025 •

edited

Loading

cwperks commented Mar 24, 2025 •

edited

Loading

nibix commented Mar 24, 2025 •

edited

Loading